Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(embedded): Guest Role Permissions #21208

Merged
merged 4 commits into from
Aug 28, 2022
Merged

fix(embedded): Guest Role Permissions #21208

merged 4 commits into from
Aug 28, 2022

Conversation

sinhashubham95
Copy link
Contributor

@sinhashubham95 sinhashubham95 commented Aug 26, 2022 •
edited
Loading

SUMMARY

This fixed the fetching of roles over a guest token. Using this the embedded dashboard can get the roles associated with the guest token.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

curl --location --request GET 'http://localhost:8080/api/v1/me/roles/'
--header 'accept: application/json'
--header 'X-guesttoken: {guest_token}'

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@sinhashubham95
Copy link
Contributor Author

@geido please check.

@codecov
Copy link

codecov bot commented Aug 26, 2022 •
edited
Loading

Codecov Report

Merging #21208 (6c63d71) into master (c61a507) will decrease coverage by 0.00%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##           master   #21208      +/-   ##
==========================================
- Coverage   66.40%   66.40%   -0.01%     
==========================================
  Files        1783     1783              
  Lines       68084    68087       +3     
  Branches     7261     7261              
==========================================
+ Hits        45214    45215       +1     
- Misses      21005    21007       +2     
  Partials     1865     1865
Flag Coverage Δ
hive 53.11% <0.00%> (-0.01%) ⬇️
mysql 81.00% <33.33%> (-0.01%) ⬇️
postgres 81.05% <33.33%> (-0.01%) ⬇️
presto 53.01% <0.00%> (-0.01%) ⬇️
python 81.53% <33.33%> (-0.01%) ⬇️
sqlite 79.57% <33.33%> (-0.01%) ⬇️
unit 50.67% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset/security/manager.py 95.07% <33.33%> (-0.30%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@sinhashubham95
Copy link
Contributor Author

@geido can you please check?

@sinhashubham95
Copy link
Contributor Author

@villebro @jinghua-qa can you please check this pr?

@sinhashubham95
Copy link
Contributor Author

@cccs-tom @dpgaspar @eschutho can you please review this pr?

@sinhashubham95
Copy link
Contributor Author

@bkyryliuk can you please check this pull request as you recently changed this file?

@sinhashubham95
Copy link
Contributor Author

@geido @hughhhh @eschutho @villebro @jinghua-qa @bkyryliuk @cccs-tom @dpgaspar can you please check this PR?

@hgrami
Copy link

hgrami commented Aug 27, 2022

Thanks for addressing this, I'm trying to implement the guest token and I'm facing the same issue. This approach is great! Hope someone reviews this PR soon.

@villebro
Copy link
Member

I started CI, will review soon

@villebro villebro changed the title security: Fixed Guest Role Permissions fix(embedded): Guest Role Permissions Aug 28, 2022
villebro
villebro approved these changes Aug 28, 2022
View reviewed changes
Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for fixing this!

@villebro villebro merged commit d408393 into apache:master Aug 28, 2022
@sinhashubham95 sinhashubham95 deleted the fix-guest-user branch August 28, 2022 11:17
@sinhashubham95 sinhashubham95 restored the fix-guest-user branch August 28, 2022 11:17
@sinhashubham95 sinhashubham95 deleted the fix-guest-user branch August 28, 2022 11:18
sadpandajoe added a commit to preset-io/superset that referenced this pull request Aug 31, 2022
@sadpandajoe
Revert "fixed guest role permissions (apache#21208)"
dd15f7a 
This reverts commit d408393.
@eschutho eschutho added the v2.0.1 label Sep 2, 2022
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@villebro villebro villebro approved these changes

Assignees
No one assigned
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XS 🚢 2.1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sinhashubham95 @hgrami @villebro @mistercrunch @eschutho @michael-s-molina